Implemented Color Mode channel for the Extended Color Light. #5762

alex-kostadinov · 2018-06-21T05:40:12Z

For all extended color lights (i.e. the 0210 ones that have color and color_temperature items), the Hue API returns information about the exact color mode the bulb is currently in. This information is used in the Hue app in order to display the RGB color palette (in case the API returned HS or XY color mode) or the color temperature palette (in case the API returned CT color mode).

Since this is important UI-related information, a channel to hold the current color mode for 0210 bulb is implemented.

lolodomo · 2018-06-21T06:59:45Z

New information channel but then what ?
As I understand, you expect a different UI depending on the mode. This new channel will not offer this feature.

Except that I don't understand how it can help, the code LGTM.

kaikreuzer · 2018-06-21T07:47:30Z

Same as @lolodomo, I do not understand the real use-case here. Independent of what mode the bulb is in, both color and color-temperature channels can be used, so UIs have the option to present the user any kind of UI. Any value that is sent as color-temperature can be shown as an HSB value on the color channel, so if the UI is in doubt, the color palette is always the safe option.
I do not think it is a good idea to store a "UI preference" within the remote device; it could also be stored in the UI itself.

alex-kostadinov · 2018-06-21T08:19:18Z

@kaikreuzer and @lolodomo On one hand, the bulb itself returns the color mode (i.e. this information comes directly form the API) and, on the other hand, since this information is available straightforward, it does not make sense to always fall back to color palette relying on the calculated HSB value.

Within the HUE app, for example, the user can choose to pick a color using a color temperature palette or a pure color palette. In the first case, the returned bulb's state denotes that the color has been set using the color temperature scheme (value is ct), while in the second case the bulb's state denotes the color has been set using the color scheme (value is hs or cie). Then, if the user navigates away and later returns back to the color set menu, either the color temperature palette or the pure color palette is rendered depending on the color mode returned in the bulb's state - at this point the user can again decide whether to select the new color value using the already rendered palette or to switch to the other one.

This is a natural UI behavior and other clients that utilize the HUE binding can make use of the color mode information. Finally, as said, this information comes in the bulb's state and I don't think it can be considered as a "UI preference".

lolodomo · 2018-06-21T14:38:29Z

This is more a device property ?
As I understand you are requesting an extend of the way the color item is handled in UI. Maybe a new kind of sitemap widget to handle color temperature ?
Or is it more a new kind of item state ?

kaikreuzer · 2018-06-21T14:40:38Z

@lolodomo I guess it is more an item that can be used in conditional visibility in sitemaps, so that either a colorpicker for the color channel is shown OR a slider for the color-temperature.

alex-kostadinov · 2018-06-22T10:08:32Z

Yes - @kaikreuzer is right - an item is utilized to be used conditionally in order to show a colorpicker or a color-temperature slider. So, @lolodomo, I believe the case is now clarified? :)

alex-kostadinov · 2018-06-22T11:00:02Z

I updated the PR with logic for creating the color_mode channel for an already paired thing of type 0210 (migration logic).

triller-telekom

One small comment from my side in the code, please have a look.

triller-telekom · 2018-06-26T15:14:15Z

...ome.binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/handler/HueLightHandler.java

@@ -125,6 +130,11 @@ private void initializeThing(@Nullable ThingStatus bridgeStatus) {
            if (getHueClient() != null) {
                if (bridgeStatus == ThingStatus.ONLINE) {
                    initializeProperties();
+
+                    if (colorModeChannelRequired()) {


What is the purpose of this logic? From the xml file I can tell that the 0210 thing has this channel, so why do you have this check and addition logic here?

kaikreuzer

the returned bulb's state denotes that the color has been set using the color temperature scheme, while in the second case the bulb's state denotes the color has been set using the color scheme

Yeah, so in the end some color is set in the bulb (and that is its state).

this information comes in the bulb's state

Can you confirm that this information is indeed stored within the bulb? Is it available in any ZLL device that is connected to the hue bridge? If so, it should be mentioned in the ZLL specification, but I cannot find anything related. I'd therefore assume that it is something that is either specific to hue bulbs OR that the information is actually not stored in the device, but maintained in the hue bridge instead.

If this is a feature of the bridge that works for all devices that are connected to it, I'd be ok to accept this channel, although it brings the risk that UIs are built that are not working for any color light (e.g. LIFX, DMX, Lightify, Hue via Zigbee) in the same way, but which require product specific code. Are you sure that this is what you want?

and I don't think it can be considered as a "UI preference"

Why not? How would you call it instead? This information has no impact at all on how the bulb looks or behaves, or am I missing something?

I updated the PR with logic for creating the color_mode channel for an already paired thing of type 0210 (migration logic).

This must not be done within the binding. Either create a separate update logic externally in your own code or help implementing #2555, so that the framework deals with it across all bindings simultaneously.

cdjackson · 2018-06-27T08:22:49Z

This is part of the ZCL standard.

Just for reference it's the issue that we had in ZigBee that I had to remove so I'm following this with interest :)

openhab/org.openhab.binding.zigbee#41
openhab/org.openhab.binding.zigbee#46

kaikreuzer · 2018-06-27T09:17:51Z

Discussion in openhab/org.openhab.binding.zigbee#41 was also a lot about how to actually SEND commands to the bulbs and it actually listed three states ("Hue and Saturation", "X and Y" and "Color Temperature"). This also is related to #4939, where we see that the binding actually needs to send commands in different ways, but that should be something internal to the binding and not be exposed to the user.

Do you possibly have a reference to the section in the spec at hand, where the exact meaning of that is described?

cdjackson · 2018-06-27T09:37:15Z

was also a lot about how to actually SEND

I think there was some confusion and linking of the issues. The SENDING of commands needs to be done as the bulb supports - some bulbs support only XY (this is mandatory) some support Hue and some support another mode (I forget the name). The binding always takes care of this and sends the command as the bulb requires.

The ZigBee issue was only about adding the color_mode channel which reflects the STATE the bulb is in.

Do you possibly have a reference

It is attribute 8 in the color control cluster -:

There is also a newer "enhanced" attribute -:

alex-kostadinov · 2018-06-28T06:09:55Z

@kaikreuzer

This must not be done within the binding. Either create a separate update logic externally in your own code

I agree and already reverted the changes considering channel creation within the lightHandler.

Can you confirm that this information is indeed stored within the bulb? Is it available in any ZLL device that is connected to the hue bridge?

As already mentioned, the only ZLL device affected by the change is the Extended Color Light (0210) and I believe @cdjackson confirmed that the color mode is stored within the bulb's state and the color mode attribute has a mandatory value in the XY range (i.e. the XY value for color mode must always be available by specification).

So, since I reverted the channel creation logic as you requested and the change does not violate the ZLL specification, can we give green light to the PR?

ivivanov-bg · 2018-06-28T06:42:18Z

@kaikreuzer

Either create a separate update logic externally in your own code or help implementing #2555

How about leaving the migration logic here and creating a follow-up task to remove it once #2555 is complete ?

(Because if not - we are blocking new features until something new is implemented - but it's not clear when /and even if/ this 'something' will ever be complete)

kaikreuzer · 2018-06-28T07:25:36Z

the change does not violate the ZLL specification

It isn't in line with the specification either as it says that there can be 3 different values.
Did you read the discussion at openhab/org.openhab.binding.zigbee#39? What is the reason that you nonetheless require this channel?

it brings the risk that UIs are built that are not working for any color light [...] Are you sure that this is what you want?

Can you confirm this, please?

How about leaving the migration logic here and creating a follow-up task to remove it once #2555 is complete ?

Definitely not, because we all know that it will stay there forever. Rather help implementing #2555 or implement such a migration logic in your own code.

ivivanov-bg · 2018-06-28T07:41:01Z

it brings the risk that UIs are built that are not working for any color light

I don't think this is correct.
After all - the newly introduced channel is a read-only and is to be used by the UI as a hint which view to show (the Color palette, or the color temperature slider). The binding should take care of the actual value based on the concrete bulb type.

It should not interfere in any way the actual API call to the bridge

kaikreuzer · 2018-06-28T12:08:07Z

Allow me to do make an alternative suggestion, which might be much more elegant and which would not interfere with how color bulbs are currently modelled in all the different bindings:

The main issue is actually that the state of the CT channel is somehow not relevant/applicable, if the bulb shows some non-white color.

In this case (where colormode=color) the state of that channel should therefore be set to NULL (meaning no state exists for it)
If a command has been sent to it (and thus colormode=ct), the CT channel has a valid state.

As a result, a UI could simply check the ct channel for NULL in order to determine whether it wants to show a colorpicker or a slider. The hue binding can internally use the color_mode attribute from the hue bridge to update the channel states accordingly.

This would allow UIs to be fully compatible with all existing bindings in a generic way and not require any additional channels. Wdyt? /cc @cdjackson, @tomhoefer

ivivanov-bg · 2018-06-28T12:17:12Z

I see at least 2 drawbacks with this approach:

When the Color of the lamp is set to a non-white (where colormode=color) - the CT channel is set to 0 or 100 - depending on the color (and not to NULL).
It is also possible the the color to be set to a particular white - orange value using the color-picker which can also be a valid CT value. In this case the CT channel will be false-positive enabled.

So - in general - this approach will be not very deterministic and there will be edge cases which will not work properly.

kaikreuzer · 2018-06-28T13:04:21Z

Why?
No, as the CT channel state would be set by evaluating the color_mode attribute.

ivivanov-bg · 2018-06-29T04:45:01Z

I don't know why - probably a bug in the HueLightHandler or in the LightStateConverter - it's just the behavior I observe on my installation
That's actually a good idea - but some changes will still be required in the handler.

Question:
How about also setting the color channel to NULL when a CT is used ?

kaikreuzer · 2018-06-29T07:12:56Z

I wasn't talking about how it currently behaves, but about my suggestion how it should behave.
Yes, again, this was my suggestion, so it still needs to be implemented if we agree on it.

How about also setting the color channel to NULL when a CT is used ?

I don't think that this makes sense, because the color channel can always have a valid state and you always want to be able to display it in a color picker (e.g. if the user switches from slider to color picker).

alex-kostadinov · 2018-06-29T08:27:23Z

@kaikreuzer Please have a look again - I reverted all unnecessary changes and only implemented a logic for setting the color_temperature channel to null if the bulb's color mode is not CT (as you proposed).

kaikreuzer · 2018-06-29T11:16:39Z

...binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/handler/LightStateConverter.java

+    public static @Nullable PercentType toColorTemperaturePercentType(State lightState) {
+        ColorMode colorMode = lightState.getColorMode();
+        if (colorMode != null && !colorMode.equals(ColorMode.CT)) {
+            return null;


Returning Java null here won't work - what you have to do is to send a channel state update UnDefType.NULL. Imho the best place to do the colorMode check would be here - simply do the update with PercentType it colorMode!=CT and otherwise do an updateState(CHANNEL_COLORTEMPERATURE, UnDefType.NULL);.
Makes sense?

Yep, it makes sense - in such a way the toColorTemperaturePercentType() method will remain untouched and will only do what it says - conversion. I will do the changes shortly.

Signed-off-by: Alexander Kostadinov <[email protected]>

kaikreuzer

Excellent, what a nice little change - thanks!

openhab-bot · 2018-10-07T18:34:18Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/philips-hue-color-temperature-not-updated-when-switching-light-on/53181/6

openhab-bot · 2018-10-07T19:03:12Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/philips-hue-color-temperature-not-updated-when-switching-light-on/53181/7

openhab-bot · 2019-02-04T12:35:38Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/philips-hue-color-temperature-not-updated-when-switching-light-on/53181/8

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch from 4e0c010 to e2f7e68 Compare June 22, 2018 10:58

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch 3 times, most recently from 3f9e4f6 to 01d6ec9 Compare June 25, 2018 08:50

triller-telekom reviewed Jun 26, 2018

View reviewed changes

kaikreuzer suggested changes Jun 27, 2018

View reviewed changes

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch from 01d6ec9 to cc99286 Compare June 28, 2018 05:36

maggu2810 added the Binding-Hue label Jun 29, 2018

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch from cc99286 to f17f1b3 Compare June 29, 2018 08:25

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch from f17f1b3 to 022eecb Compare June 29, 2018 10:35

kaikreuzer suggested changes Jun 29, 2018

View reviewed changes

Implemented Color Mode channel for the Extended Color Light.

2e0ee32

Signed-off-by: Alexander Kostadinov <[email protected]>

alex-kostadinov force-pushed the feature/hue/implement-color-mode branch from 022eecb to 2e0ee32 Compare June 29, 2018 11:36

kaikreuzer approved these changes Jun 29, 2018

View reviewed changes

kaikreuzer merged commit 0c0de9b into eclipse-archived:master Jun 29, 2018

alex-kostadinov deleted the feature/hue/implement-color-mode branch July 1, 2018 10:51

triller-telekom mentioned this pull request Jul 9, 2018

[OneWire] Initial contribution #4287

Merged

5 tasks

htreu added this to the 0.10.0 milestone Oct 30, 2018

TomTravaglino mentioned this pull request Dec 3, 2018

Recognize color mode of color bulb openhab/org.openhab.binding.zigbee#326

Closed

TomTravaglino mentioned this pull request Dec 13, 2018

Recognize color mode of color bulb openhab/org.openhab.binding.zigbee#336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented Color Mode channel for the Extended Color Light. #5762

Implemented Color Mode channel for the Extended Color Light. #5762

alex-kostadinov commented Jun 21, 2018 •

edited

Loading

lolodomo commented Jun 21, 2018

kaikreuzer commented Jun 21, 2018

alex-kostadinov commented Jun 21, 2018

lolodomo commented Jun 21, 2018

kaikreuzer commented Jun 21, 2018

alex-kostadinov commented Jun 22, 2018

alex-kostadinov commented Jun 22, 2018

triller-telekom left a comment

triller-telekom Jun 26, 2018

kaikreuzer left a comment

cdjackson commented Jun 27, 2018

kaikreuzer commented Jun 27, 2018

cdjackson commented Jun 27, 2018

alex-kostadinov commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018 •

edited

Loading

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 29, 2018

kaikreuzer commented Jun 29, 2018

alex-kostadinov commented Jun 29, 2018

kaikreuzer Jun 29, 2018

alex-kostadinov Jun 29, 2018

kaikreuzer left a comment

openhab-bot commented Oct 7, 2018

openhab-bot commented Oct 7, 2018

openhab-bot commented Feb 4, 2019

Implemented Color Mode channel for the Extended Color Light. #5762

Implemented Color Mode channel for the Extended Color Light. #5762

Conversation

alex-kostadinov commented Jun 21, 2018 • edited Loading

lolodomo commented Jun 21, 2018

kaikreuzer commented Jun 21, 2018

alex-kostadinov commented Jun 21, 2018

lolodomo commented Jun 21, 2018

kaikreuzer commented Jun 21, 2018

alex-kostadinov commented Jun 22, 2018

alex-kostadinov commented Jun 22, 2018

triller-telekom left a comment

Choose a reason for hiding this comment

triller-telekom Jun 26, 2018

Choose a reason for hiding this comment

kaikreuzer left a comment

Choose a reason for hiding this comment

cdjackson commented Jun 27, 2018

kaikreuzer commented Jun 27, 2018

cdjackson commented Jun 27, 2018

alex-kostadinov commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018 • edited Loading

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 28, 2018

kaikreuzer commented Jun 28, 2018

ivivanov-bg commented Jun 29, 2018

kaikreuzer commented Jun 29, 2018

alex-kostadinov commented Jun 29, 2018

kaikreuzer Jun 29, 2018

Choose a reason for hiding this comment

alex-kostadinov Jun 29, 2018

Choose a reason for hiding this comment

kaikreuzer left a comment

Choose a reason for hiding this comment

openhab-bot commented Oct 7, 2018

openhab-bot commented Oct 7, 2018

openhab-bot commented Feb 4, 2019

alex-kostadinov commented Jun 21, 2018 •

edited

Loading

ivivanov-bg commented Jun 28, 2018 •

edited

Loading